feat(dips): switch to offer-based RCA authorization#1009
Draft
MoonBoi9001 wants to merge 5 commits intomb9/dips-price-rejection-loggingfrom
Draft
feat(dips): switch to offer-based RCA authorization#1009MoonBoi9001 wants to merge 5 commits intomb9/dips-price-rejection-loggingfrom
MoonBoi9001 wants to merge 5 commits intomb9/dips-price-rejection-loggingfrom
Conversation
The indexing-payments-management-audit-fix-reduced contract branch adds a uint16 conditions bitmask to the RCA struct between maxSecondsPerCollection and nonce. The bitmask enables payer-declared conditions like eligibility checks. Threaded through the sol! type in crates/dips/src/lib.rs and all six test fixtures. The EIP-712 typehash is derived automatically by the sol! macro once the struct shape matches, so no pinned typehash string to update here (unlike dipper). Defaults to 0 (no conditions). Setting CONDITION_ELIGIBILITY_CHECK = 1 requires a contract payer that implements the eligibility callback interface -- against an EOA payer the on-chain offer() and accept() calls revert. The derive_agreement_id preimage is unchanged; the contract's _generateAgreementId does not hash conditions. Confirmed via the pinned shared test vector which matches byte-for-byte with dipper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Indexer-service no longer verifies RCA proposals via EIP-712 signature recovery and escrow-authorized signer lookup. Instead, it validates that an on-chain RCA offer exists by querying the graphprotocol/indexing-payments-subgraph for the corresponding Offer entity and comparing the stored offerHash to the locally-computed hashRCA(rca). Changes: - New crates/dips/src/offers.rs module with OfferLookup trait, HttpOfferLookup production impl (queries subgraph via raw GraphQL POST), and MockOfferLookup test impl with a preload-friendly API. - Deleted crates/dips/src/signers.rs (SignerValidator trait, EscrowSignerValidator wrapper, Noop/RejectingSignerValidator stubs). The underlying EscrowAccountsWatcher in crates/monitor stays in place because TAP still uses it for payer-signer authorization. - SignedRecurringCollectionAgreement::validate is now async and takes &Arc<dyn OfferLookup> instead of &Arc<dyn SignerValidator>. On an empty signature wire payload, it skips signature recovery entirely and branches on offer_lookup.get_offer_hash(agreement_id) results. - DipsError drops SignerNotAuthorised and adds OfferNotFound, OfferMismatch, and OfferLookupFailed. reject_reason_from_error maps all three to the existing gRPC RejectReason::SignerNotAuthorised variant for wire-protocol compatibility -- dipper's response handler only distinguishes Accept vs Reject, not between individual Reject reasons, so no proto change is needed in this release. - DipsServerContext swaps the signer_validator field for offer_lookup. - DipsConfig gains indexing_payments_subgraph_url (required, Url type) and service.rs plumbs it into HttpOfferLookup on startup. - crates/config/maximal-config-example.toml gains the new field and the test_maximal_config fixture is updated to match. - All 57 indexer-dips unit tests updated to use MockOfferLookup with pre-seeded (agreement_id -> hash) pairs. Added two new tests: test_validate_and_create_rca_offer_not_found (empty lookup) and test_validate_and_create_rca_offer_mismatch (seeded with wrong hash). - The old test_validate_and_create_rca_unauthorized_signer test is removed (replaced by the two offer-path tests above). The wire-level ABI encoding is unchanged: dipper sends a SignedRecurringCollectionAgreement wrapper with the bytes signature field present but empty. The contract takes the offer-path branch in _requireAuthorization when the indexer-agent forwards this payload to SubgraphService.acceptIndexingAgreement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The gRPC handler checked the indexing-payments-subgraph for an on-chain offer before accepting a proposal. This is wrong: the proposal is a negotiation step where the indexer validates pricing and metadata. The on-chain offer doesn't exist yet — dipper submits it only after getting Accept. The contract enforces offer existence at acceptIndexingAgreement time, not at proposal time. Delete offers.rs (OfferLookup trait, HttpOfferLookup, MockOfferLookup), remove the offer_lookup field from DipsServerContext, simplify validate() to only check serviceProvider, remove OfferNotFound/OfferMismatch/ OfferLookupFailed error variants, and drop indexing_payments_subgraph_url from DipsConfig. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The validate() docstring had leftover text from the intermediate offer-lookup iteration that contradicted the proposal-first description directly below it, and pointed at an offers::OfferLookup module that no longer exists. Tests were left carrying rca_eip712_domain, derive_agreement_id, and eip712_signing_hash bindings that the new validation path does not consume. Drop those along with the now-unused CHAIN_ID constant and SolStruct import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
encode_empty_sig was the test-only helper for building RCA payloads during the offer-based authorisation iteration, where the empty-sig path meant something. After the revert to proposal-first validation the validator does not consume the signature at all, so the name implies a distinction that no longer exists. Rename to rca_to_wire_bytes, which describes what the helper produces without suggesting anything about how the payload will be validated. Add test_rca_conditions_field_roundtrip guarding the sol! struct's uint16 conditions field against layout drift: encode with a non-zero conditions value, decode, assert the value survives and that the neighbouring maxSecondsPerCollection and nonce slots are intact so a future misalignment is not misread from an adjacent field. This is the field audited into the audit-branch contract, so a silent decode drift would produce RCAs whose hash diverges from the contract's view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Today indexer-service's DIPs validator takes the signature from an incoming
SignedRecurringCollectionAgreement, recovers the signer, and checks it against the authorized signer set by querying the network subgraph for escrow accounts. This mirrors the TAP path and reusesEscrowSignerValidator. If the signer is not in the authorized set, indexer-service rejects the proposal asSIGNER_NOT_AUTHORISED.The audit-bound contracts branch (graphprotocol/contracts#1312) introduces a second authorization path. Under this path the payer submits an
offer()transaction toRecurringCollector, and acceptance compares the stored hash against the locally computed one instead of recovering a signature. The same branch adds auint16 conditionsfield toRecurringCollectionAgreement, which must also be threaded through the mirroring sol! struct here or the struct hash will not match what the contract expects.The end-to-end migration runs proposal-first: dipper sends the gRPC proposal, indexer-service accepts or rejects on terms alone (price, metadata, network, deadline, service-provider match), and only on Accept does dipper submit the
offer()transaction. The contract then enforces offer existence atacceptIndexingAgreementtime via its ownrcaOffers[agreementId]storage mapping — that check lives in the contract, not in the gRPC handler. Consequently indexer-service no longer needs to know about signer authorization or on-chain offers during proposal validation, and the subgraph-based offer lookup that was briefly added to this PR (commit0c02250) was reverted in047e0ebbecause the offer does not exist yet at proposal time.A side effect is that idle indexers without escrow accounts are no longer blanket-rejected as
SIGNER_NOT_AUTHORISED— the escrow set is no longer consulted on the DIPs path at all. The trade-off is that indexer-rs relies on the contract, not the service layer, to stop gas-burning accept attempts on missing offers. Tracked as a defence-in-depth follow-up in graphprotocol/indexer#1198.Summary
uint16 conditionsto the sol!RecurringCollectionAgreementstruct betweenmaxSecondsPerCollectionandnonce, matching the audit-branch layout. The field is intentionally excluded from theagreementIdpreimage — the contract derives the id from(payer, dataService, serviceProvider, deadline, nonce)only.crates/dips/src/signers.rsentirely — theSignerValidatortrait,EscrowSignerValidator,NoopSignerValidator,RejectingSignerValidator, and associated tests are all gone.signer_validatorfield fromDipsServerContext, signature recovery fromSignedRecurringCollectionAgreement::validate, and theSignerNotAuthorisedinternal error variant. The gRPCRejectReason::SignerNotAuthorisedenum value is retained for wire compatibility but no error path maps to it.validateis no longer async; it performs a single service-provider equality check.EscrowSignerValidatorwiring fromcrates/service/src/service.rs.encode_empty_sighelper; removederive_agreement_id/rca_eip712_domain/eip712_signing_hashbindings that the new validation path does not exercise.Stack
Stacked on #990 (
mb9/dips-price-rejection-logging). Paired PRs: edgeandnode/dipper#607 and graphprotocol/indexing-payments-subgraph#4. Paired contracts branch:indexing-payments-management-audit-fix-reduced(graphprotocol/contracts#1312).Follow-up: graphprotocol/indexer#1198 (agent-side
rcaOfferspre-check to harden the proposal-first flow against malicious or absent payers).Draft until end-to-end verification against local-network is complete.